Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to eliminate warnings on RTD doc generation #1371

Merged
merged 4 commits into from
Aug 24, 2021
Merged

Updates to eliminate warnings on RTD doc generation #1371

merged 4 commits into from
Aug 24, 2021

Conversation

swcurran
Copy link
Contributor

Signed-off-by: Stephen Curran [email protected]

I added a mock for aries_cloudagent.vc, which eliminated all of the errors. Of course I suspect that it also removes the doc generation for that module, but am not certain. But I feel better that all the warnings are gone...

@andrewwhitehead
Copy link
Contributor

My other PR should fix most of the warnings I think, if you want to test docs generation there.

@swcurran
Copy link
Contributor Author

Pretty sure I did, but I can try again.

@swcurran
Copy link
Contributor Author

I'm guessing you are talking about #1366? I ran the generation on top that and got many of the same errors. Once I mocked the one ACA-Py module (vc) on top of that PR, all but one warning (below) went away. That said, I need to update this PR after the PR is merged, as the RST generator needs to run on the latest code. It would be nice to have that run on every build and added (if needed) to each PR. Not sure how to do that...

The remaining error is this. Is this a code error?

WARNING: error while formatting arguments for aries_cloudagent.protocols.issue_credential.v1_0.models.credential_exchange.V10CredentialExchange: unhashable type: 'list'

@andrewwhitehead
Copy link
Contributor

I need more context to figure out what's causing that error, and I'm not set up to test it right now. If the errors are just warnings then I'm not sure we need to disable documentation for that module.

@codecov-commenter
Copy link

Codecov Report

Merging #1371 (cd1294b) into main (a304568) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1371   +/-   ##
=======================================
  Coverage   95.40%   95.40%           
=======================================
  Files         476      476           
  Lines       28969    28969           
=======================================
  Hits        27637    27637           
  Misses       1332     1332           

@swcurran
Copy link
Contributor Author

I've updated this branch with your merged PR and all is good now.

We're still generating the docs for that module (vc) AFAIK -- we're just not allowing it to be referenced from where it is called. Not sure why we have to do that. FYI -- there had been two other modules for which we used to do that, but I just tested them and that is no longer needed.

The one error is left, but I can't figure out why it is happening. I checked various things to do with Sphinx, but can't trace it. I'm a bit worried it's a code issue, but that's beyond me.

This is now ready to merge.

@swcurran
Copy link
Contributor Author

@ianco -- is this integration error anything to do with my changes (seems very unlikely...) or an intermittent error?

Do I fake a commit to retrigger?

@ianco ianco enabled auto-merge August 24, 2021 20:20
@ianco ianco merged commit 39b0a8a into openwallet-foundation:main Aug 24, 2021
@ianco
Copy link
Contributor

ianco commented Aug 24, 2021

@ianco -- is this integration error anything to do with my changes (seems very unlikely...) or an intermittent error?

Do I fake a commit to retrigger?

This is an intermittent error. You can re-run the rests from the workflow page (click on the details link and then there should be a button to rerun the workflow)

@swcurran swcurran deleted the rtd-updates-071 branch August 25, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants